Skip to content

Fix Rust breaking changes (removal of RoomListItem)#181

Merged
bnjbvr merged 4 commits into
mainfrom
bnjbvr/get-rid-of-another-room
May 26, 2025
Merged

Fix Rust breaking changes (removal of RoomListItem)#181
bnjbvr merged 4 commits into
mainfrom
bnjbvr/get-rid-of-another-room

Conversation

@bnjbvr

@bnjbvr bnjbvr commented May 22, 2025

Copy link
Copy Markdown
Contributor

Also a few unrelated goodies for generally helping with this repo maintenance.

Rust PR.

@bnjbvr

bnjbvr commented May 22, 2025

Copy link
Copy Markdown
Contributor Author

I think the test is failing because the timeline created by Room::timeline is now not cached anymore in the Room instance; as a result, it could be that the fresh timeline reconstructed has minimal events, because sync sent a gap, and the timeline was reset as a result. If that's the case, then one of two things must be done about this:

  • backpaginate with the fresh timeline recreated after the sending
  • make sure to use the same timeline instance along the test

@bnjbvr

bnjbvr commented May 26, 2025

Copy link
Copy Markdown
Contributor Author

For what it's worth, it seems that Tests / Tests (viz., Ensuring client implementations work) now passes, after a first failed attempt.

I wonder if the live_events_rust test may be intermittently failing, because of the following:

  • the Rust SDK is being created, and sync is started on it
  • there's an RPC client that will create a room and send messages to it
  • the Rust SDK is then being polled to see if it's received the events (through a timeline attached to the new room)

In some cases, it seems the Rust SDK doesn't emit timeline diff updates for all the events in the room. I think this may happen because the server marked a sliding sync as limited/gappy, so the SDK may have reset the timeline, resulting in a limited view over the timeline's events. It's not possible to tell if that happened, from looking at the current logs (and unfortunately I can't run this test locally, as I'm running into strange unmarshall errors, maybe related to using google-chrome at a different version than the one expected here).

Some events appear as missing, but they should be present in the persistent storage. If a subsequent back-pagination happened they'd be immediately reloaded from the store, if they've been seen; or correctly reloaded from the network, if the server decided to return only a subset of the events for any reason. The above test existing_events does run a back-pagination, with a comment meaning that it could be that some events are missing; I think that with the persistent storage, it's easier to assume the same thing, even in the case of live events, as a reset may happen, depending on how the events are received from the sync (in a single batch or in multiple batches, was the room subscribed to, etc.).

Of course, I might be wrong here, and there might be a bug where the client is missing some events, but this sounds rather unlikely. I'm tempted to add a back-pagination there too, in case we're hitting this edge-case, but maybe we can see how this works out first, in general.

@Hywan Hywan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good. I'm okay with the way the timeline is cached and the place where it is computed.

Thanks!

@bnjbvr bnjbvr merged commit 28e6eea into main May 26, 2025
@richvdh richvdh mentioned this pull request Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants